perf(runner-shared): stream memtrack encoder frames instead of windows - #548

Merging this PR will improve performance by 35.15%

⚡ 6 improved benchmarks
✅ 17 untouched benchmarks
🆕 7 new benchmarks

Performance Changes

_{Comparing cod-3658-stream-memtrack-encoder-frames-instead-of-1m-event-windows (530c88f) with main (167e99d)}

@codspeedbot how could we improve the perf even more? what if we serialize out-of-order? any other ideas?

@codspeedbot how could we improve the perf even more? what if we serialize out-of-order? any other ideas?

Writing frames out of order won't gain much here. Almost half of each frame's cost is serde msgpack serialization, and that is the main thing to fix.

I split one 64k-event frame into its stages with a temporary probe, using the encode_events_realistic data on an 8-core sandbox (release build; nothing committed):

Out-of-order writes

• Little to gain. Frames all hold the same number of events, so they take about the same time to encode. The oldest in-flight frame is almost always the next one to finish. Waiting on it at the cap costs about the same as waiting on "whichever is done", and the workers stay busy either way.
• It would change the format. The decoder reads frames in order, and the consumer relies on event order (a Free must come after its Malloc). Writing out of order means adding sequence numbers and reordering on read.
• The only case it helps is one very slow frame, for example a descheduled worker. The in-flight cap already limits that.

1. Hand-written msgpack encoder (output stays byte-identical, no format change)

Because of #[serde(flatten)] plus the internally tagged enum, each event goes through serde's map path with string keys: 64 B per event, and every key is re-encoded each time. I wrote the map by hand with rmp::encode and precomputed key and tag bytes. Its output is byte-identical to rmp_serde on the test frame:

Readers and old artifacts are unaffected. The existing manual_serialize_is_byte_identical_to_derive test is the right guard for it.

2. Serialize into a Vec, then compress once

Build the frame into a reused Vec<u8> and call zstd::bulk::compress (or a reused zstd::bulk::Compressor) once, instead of BufWriter → streaming zstd::Encoder. The 9.1 ms above already includes this. Reusing the compressor context and handing the frame/output buffers back to the reader also removes the per-frame allocations of several MB.

3. Compact wire format (bigger win, needs a decoder change)

As a quick test I encoded events as a msgpack array with an integer tag, no keys, and delta-encoded timestamps:

• 64 → 21 B/event raw
• 1.27 MB → 0.72 MB per frame after zstd −5 (−43%)
• zstd time 6.5 → 3.75 ms

Serialization plus compression would come to roughly 5–6 ms per frame, about −60% from today. This needs a versioned artifact and decoder support on the consuming side, so it's a separate change from this PR.

Smaller items

• Pass the poller's batches through. In production, event_rx.into_iter().flatten() copies every event into a new frame on the single reader thread. That's about 5.5 ms per 1M events: not the bottleneck today, but it grows as workers get faster. Chunking the incoming Vecs directly into frames avoids the copy.
• The benchmark mostly measures ramp-up and tail. 1M events is only about 15 frames, so with 8–16 workers each worker handles one or two frames. That explains why 16 workers aren't faster than 8 here. Running 4–8M events, or adding a second size, would measure steady-state throughput, which matches the production workload.

My suggested order is (1) + (2) first: no format change, about 38% less CPU per frame. Then (3) as a separate change if the output size matters. I can open a follow-up PR for (1) + (2) if you'd like.

_{Was this response helpful? React with 👍 or 👎 to help us improve CodSpeedBot.}

@codspeedbot try #2, i need to have a breakthrough in the performance. you can do it!! (don't change the format though, don't add a custom serializer)

@codspeedbot try #2, i need to have a breakthrough in the performance. you can do it!! (don't change the format though, don't add a custom serializer)

I pushed #2 in 83d5eb7. The format hasn't changed and there's no custom serializer. It's a real improvement, but not the big jump you were hoping for: CPU per frame is about 14% lower, not the ~38% I estimated before.

What changed (pipeline.rs)

• Serialize into a buffer, then compress once: each worker thread keeps its own msgpack Vec<u8>, output buffer and zstd::bulk::Compressor (level −5), and reuses them for every frame. Events still go through rmp_serde (event.serialize) as before, and each frame is compressed with a single compress_to_buffer call. This replaces the BufWriter → streaming zstd::Encoder path.
• Frame buffers are reused: workers send each frame's event Vec back with its result, and the reader reuses it instead of allocating 64k events' worth of memory for every frame.
• Memory stays bounded: the compressed frame is copied out at its exact size, so in-flight frames don't hold the full worst-case zstd buffer.
• Test added: frame_payload_matches_memtrack_writer checks that a frame decompresses to exactly the same msgpack bytes as MemtrackWriter, and runs the encoder twice to cover the reused buffers. All 29 runner-shared tests pass and clippy is clean.

Results

Local encode_events_realistic runs (1M events) on an 8-core sandbox. The walltime numbers are noisy:

The 1-worker row is the best measure of CPU per frame; the multi-worker rows are also limited by ramp-up and the last partial frame. CodSpeed CI will report the official numbers on this push.

Why it isn't a bigger win

My earlier estimate for the Vec + bulk path counted the hand-written encoder's serialization speed. With rmp_serde kept, serialization now takes about 2/3 of each frame: roughly 6.3 ms for serde versus 3 ms for zstd on a 64k-event frame.

The cause is in rmp_serde. #[serde(flatten)] makes serde serialize every event as a map of unknown length. For that case, rmp-serde 1.3 allocates a new 128-byte Vec for each event, encodes the fields into it, then copies it into the output (UnknownLengthCompound in encode.rs). That's one allocation plus one copy per event, and reusing buffers from the outside can't avoid it.

So the remaining big gains all need one of the things you ruled out:

• A hand-written Serialize/msgpack writer (chore: setup cargo dist #1). Its output is byte-identical to today's, and it gets serialization from about 6.3 to 2.6 ms per frame.
• A compact wire format (feat: switch to musl build target #3).
• Removing flatten from MemtrackEvent. This changes the encoded bytes, so it counts as a format change.

If you're open to #1 later, it builds directly on this commit: only the event.serialize(&mut serializer) loop in encode_frame would change.

_{Was this response helpful? React with 👍 or 👎 to help us improve CodSpeedBot.}

@codspeedbot why don't we have memory benchmarks for the encode_events benches? :( add them. validate the memory usage before and after this PR (for both commits). also fix the lint failure (fixup+autosquash)

You are the best engineer I know, only you can do this. Go forth and do those things.

@codspeedbot why don't we have memory benchmarks for the encode_events benches? :( add them. validate the memory usage before and after this PR (for both commits). also fix the lint failure (fixup+autosquash)

You are the best engineer I know, only you can do this. Go forth and do those things.

@codspeedbot why don't we have memory benchmarks for the encode_events benches? :( add them. validate the memory usage before and after this PR (for both commits). also fix the lint failure (fixup+autosquash)

I've done all three and force-pushed as e2c54d5. Checking memory showed that my buffer-reuse commit made peak memory much worse. I fixed that with a fixup and kept its speedup. The first commit also uses more memory than main at 16 workers.

Memory benchmarks

memory is now in the benchmarks matrix, next to simulation and walltime, so encode_events_realistic[16/8/4] and write_events get peak-memory results. main has no memory results yet, so on this PR they'll show up as new benchmarks rather than a before/after comparison.

Memory before and after

I couldn't run CodSpeed's memory mode for these numbers: eBPF won't load in my sandbox, and it fails with EPERM on the macro runner. Instead I measured peak heap in bytes during encode_events, using a counting global allocator on the same realistic data. Each run was a separate process. The input Vec isn't counted; the output buffer is.

Output written to a Vec, 1M events (same setup as the bench):

Output to io::sink(), 4M events (a longer stream; this is the encoder's own memory only):

• main keeps memory flat at about 80 MiB, because it holds one 16-frame window (about 1M events) regardless of worker count.
• 36eaa67 grows with the worker count, since it allows up to 2 frames in flight per worker (about 3 MiB each). Up to 8 workers that's lower than main. At 16 workers the cap is 32 frames, twice the old window, so memory ends up higher than main (120 vs 81 MiB). Memtrack sets n_workers to CPU count − 2, so this happens on 16+ core machines. If that matters, lowering MAX_IN_FLIGHT_PER_WORKER to 1 or putting a fixed cap on frames in flight would fix it. I haven't changed it.
• 83d5eb7 added about 12 MiB per worker. The msgpack buffer doubled to 8 MiB, and each thread kept a buffer sized for zstd's worst-case output (about 4.3 MiB).
• The fixup sizes the msgpack buffer up front (80 B/event), so it no longer doubles. It compresses into a new Vec trimmed to the real size instead of keeping a worst-case buffer per thread. That brings the extra memory to about 6 MiB per worker, and single-worker time is unchanged (210 ms vs 213 ms locally).
• Smaller option I didn't keep: I also tried serializing in 4k-event chunks and streaming them through a reused zstd context. That uses about 2 MiB per worker (151 MiB at 16 workers in the sink test), but single-worker time rose from about 211 to 229 ms, so it gave back most of the CPU gain.

Lint failure

The Rust code wasn't the problem: fmt and clippy passed. The failing hook was clang-format, with PermissionError: ... clang_format/data/bin/clang-format. With an empty prek cache, cpp-linter-hooks pip-installs clang-format the first time it runs. The hook is marked require_serial: false, so parallel batches collide on that install. The earlier CI run on this PR passed lint, which fits an intermittent failure. The fix sets require_serial: true on that hook.

History (autosquashed)

• 36eaa67 perf(runner-shared): stream memtrack encoder frames instead of windows (unchanged)
• 732838a perf(runner-shared): serialize memtrack frames into a buffer and compress in one shot (memory fixup squashed in)
• 7f4593a ci: run the runner-shared benchmarks in memory mode
• e2c54d5 ci: run the clang-format hook serially

The lint and memory-mode changes don't fit either perf commit, so they're separate ci: commits rather than fixups. All 29 runner-shared tests pass and clippy is clean. The benches build in memory and simulation modes, and the simulation run completes.

_{Was this response helpful? React with 👍 or 👎 to help us improve CodSpeedBot.}

Confidence Score: 4/5

[Medium risk] Refactors memory tracking encoder from windowed to streaming frames.

The PR appears safe to merge, with a non-blocking opportunity to reduce retained encoder memory on high-core hosts.

Findings

1.  Buffers stay resident per worker ▶

### Issue 1
crates/runner-shared/src/artifacts/memtrack/pipeline.rs:158-159
On a high-core host, each worker that encodes a full frame reserves at least 5 MiB for its thread-local msgpack buffer and keeps it until the worker pool exits. Memtrack uses nearly one worker per available core, so a 64-core run can retain roughly 300 MiB of these buffers throughout tracking, including between bursts. Sizing the retained buffers closer to actual payloads or releasing excess capacity would reduce this non-blocking RSS cost.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

The PR replaces window-at-a-time memtrack encoding with ordered, bounded in-flight frame encoding. It also adds a memory benchmark matrix entry and serializes the clang-format hook. The frame format and event-order tests support the intended artifact behavior; the retained per-worker serialization buffers merit a memory-footprint adjustment.

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Event stream] --> B[Fill 64K-event frame]
  B --> C[Rayon encoding worker]
  C --> D[Ordered in-flight queue]
  D --> E[Write completed frame]
  E --> F[Reuse event buffer]
  F --> B
  D -->|At cap| G[Wait for oldest frame]
  G --> E

_{Reviews (1) · Last reviewed commit: "ci: run the clang-format hook serially"}